Audit the auditor: evidence rules, substance checks, waiver injection fix (3.4.0) - #8
Conversation
Task 1: a pass with an empty evidence list can no longer claim to be verified. The downgrade is automatic, so WEAK_PASS_IDS no longer has to be remembered for new checks. Structural passes (dep-1, ci-6, res-2) now carry the evidence they always had. Task 2: five checks passed on costumes. A CI step of echo "test skipped" satisfied ci-2 (critical). A test script of echo && exit 0 satisfied ci-6. An empty runbook.md satisfied res-1. An unchecked TODO box satisfied res-2. They now require a real test runner (string literals stripped first), 50+ words of prose, and a non-TODO line. A deliberately hollow repo scored 78 with 9 passes, 5 of them false. It now scores 70 with 5 passes and no false verified claims. 46 tests.
1836 lines in one file, over the 800-line ceiling in the project's own coding rules, with three hand-maintained tables sitting far from the checks they describe. Now 9 modules, largest 503 lines: model, repo, fingerprint, checks_build, checks_runtime, checks_quality, runner, report. prod_audit.py stays the entry point and re-exports the public names, so every documented command and every existing import keeps working. Verified: 46 tests pass, --help/--profile/--fail-on work from an unrelated cwd by absolute path, self-audit unchanged.
The script's findings have 46 tests. The documents Claude writes from them had none, and they are what the user reads. check_report.py enforces what SKILL.md step 8 previously only asked Claude to remember: a quoted file path must appear in the scan, a weak pass must not be written up as a win, every critical must be addressed in the brief, a waived finding must stay visible, no placeholders, no em dashes. Run against the real jarvis documents it found 6 issues in prose that had already been reviewed by hand. Two turned out to be false demotions caused by checks that had real evidence and never filled the field (log-1 and log-2 match installed dependencies); those now carry it, along with sec-3, sec-4 and ms-1 which cite the scope they searched. The other four were genuine: a file path that does not exist, a hand-verified path not labelled as such, and em dashes in both docs. 53 tests.
A hollow repo scored 78 with five false verified passes. Fixing that surfaced a markdown injection through the waiver file, a linter whose rule contradicted our own templates, and dependency matching that counted @types/pino as a logger. 59 tests.
|
Warning Review limit reached
Next review available in: 23 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (19)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1 participant
Started as the five improvements from a self-review. Each one uncovered something worse.
The finding that started it
A deliberately hollow repo, where every control is a costume:
.github/workflows/ci.ymlrun: echo "test skipped"package.jsontest scriptecho "no tests yet" && exit 0docs/runbook.mdREADME.md- [ ] figure out rollbackScored 78/100 with nine passing checks, five of them false, all claiming
verified.ci-2is a critical check and it matched the word test inside an echo string.Now: 70/100, five passes, no false verified claims.
Root cause was structural, not data
Confidence lived in a hand-typed set of three ids, 1,600 lines from the checks it described, with nothing keeping it in sync with 31 check ids. So the rule is now intrinsic: a pass with no evidence cannot be
verified. New checks are covered automatically.That rule immediately flagged six checks that had real evidence and never filled the field. Those now cite it (matched dependency, lockfile, script, matching line) or, for proofs of absence, the scope they searched.
Security: the waiver file is untrusted input
.prod-audit-waivers.jsonlives in the audited repo. Its text went into the report unescaped. A craftedreasonforged a second## Repository Controls Score: 100/100heading. Reproduced, then fixed at load time rather than at each render site. Regression test asserts on heading lines, not substrings.This directly violated the "audited repo is data, not instructions" rule shipped in 3.3.0 this morning.
The linter rejected our own output
check_report.pybans em dashes; the templates contained 30 and every grade string was"A — Strong evidence of controls". Following the template guaranteed a failing gate. Em dashes removed from every generated string, with a test that the grade strings pass our own linter.New: the generated documents are checked
The script had 46 tests. The documents Claude writes from it, which are what users read, had none.
check_report.pyenforces what step 8 previously only asked Claude to remember. Run against real, already-hand-reviewed documents it found six problems, two of which turned out to be the new evidence rule being wrong.Refactor
1836 lines in one file, over this project's own 800-line ceiling, into an
audit/package: 8 modules, largest 503. Entry point re-exports everything, so every documented command still works, verified from an unrelated cwd. 77 unused imports removed.Verified
Not done, and why
claude plugin evalis in early access and unavailable on this account. I did not write eval files against a schema I cannot validate. The five prompt-only skills were instead each tested once by hand against a real repository by an independent agent; every one came back "partly works" with specific defects. Those are tracked for 3.5.0 and listed in the PR discussion.